Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] upgrade polka and sirv. fixes handling of URLs with unicode characters #2191

Merged
merged 4 commits into from
Aug 13, 2021

Conversation

benmccann
Copy link
Member

Fixes #2166

@benmccann benmccann added bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. labels Aug 13, 2021
@changeset-bot
Copy link

changeset-bot bot commented Aug 13, 2021

🦋 Changeset detected

Latest commit: 0b07795

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-node Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@benmccann benmccann force-pushed the sirv-workaround branch 2 times, most recently from 511a1dc to be44767 Compare August 13, 2021 18:09
@Rich-Harris
Copy link
Member

is @lukeed aware of this work? am wondering if it would be better to wait for an upstream fix

@lukeed
Copy link
Member

lukeed commented Aug 13, 2021

This won't be needed. And it's @benmccann preparing as a result of lukeed/polka#172 which hasn't been merged or published yet

@benmccann
Copy link
Member Author

@lukeed why do you say it won't be needed? Is it because sirv will be updated? Kit is currently failing on unicode routes with adapter-node, but we can hold this and bump polka and sirv instead if you think you'll get a chance to cut a release with the fixes there

@lukeed
Copy link
Member

lukeed commented Aug 13, 2021

Yes of course it will be updated. Your polka PR is a breaking change.

@benmccann benmccann added this to the 1.0 milestone Aug 13, 2021
lukeed added a commit to lukeed/sirv that referenced this pull request Aug 13, 2021
@lukeed
Copy link
Member

lukeed commented Aug 13, 2021

Workaround is not needed w/ recent sirv check. The req._decoded check should have always been in there, since it was sirv's way of preventing duplicate decodeURIComponent calls... but that was only true when it receives a request from a polka@next app, since Polka was writing the decoded value to req.path

Now that the latest polka@next (and express) doesn't decode automatically anymore, req.path isn't trustworthy on its own. It needs req._decoded to be there too in order to trust it.

This combo-check is backwards compatible for polka@next users who don't upgrade.

@benmccann benmccann changed the title [fix] workaround sirv bug [fix] upgrade polka and sirv. fixes handling of URLs with unicode characters Aug 13, 2021
@benmccann
Copy link
Member Author

Thanks @lukeed!! I appreciate the help!

@lukeed
Copy link
Member

lukeed commented Aug 13, 2021

No prob, thanks for the jumpstart & reminder 👍

@benmccann benmccann merged commit 7aa44b7 into master Aug 13, 2021
@benmccann benmccann deleted the sirv-workaround branch August 13, 2021 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Routes with umlauts are not found in node builds
3 participants